feat(core): add OAuth2 consent flow support - #584

packages/nextjs/src/pages/client.ts (1)

27-29: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Remove redundant and unsafe window.location.origin access.

guessPotentiallyProxiedOrySdkUrl already safely defaults to window.location.origin when running on the client. Accessing window.location.origin unconditionally here bypasses that safety and will throw a ReferenceError if this factory is ever accidentally invoked during Server-Side Rendering (SSR).

♻️ Proposed refactor

-      basePath: guessPotentiallyProxiedOrySdkUrl({
-        knownProxiedUrl: window.location.origin,
-      }),
+      basePath: guessPotentiallyProxiedOrySdkUrl(),

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/nextjs/src/pages/client.ts` around lines 27 - 29, Remove the
explicit knownProxiedUrl value from the guessPotentiallyProxiedOrySdkUrl call in
the client factory, allowing the helper to apply its safe client-side default
without unconditionally accessing window during SSR.

packages/nextjs/src/pages/consent.ts (1)

76-79: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider logging the error before swallowing it.

Returning null on failure handles invalid challenges gracefully, but silently swallowing all errors (such as 500s or misconfigured API keys) can make debugging difficult. Consider logging the error to help developers identify configuration or network issues.

♻️ Proposed refactor

   return serverSideOAuth2Client()
     .getOAuth2ConsentRequest({ consentChallenge })
-    .catch(() => null)
+    .catch((error) => {
+      console.error("Failed to fetch OAuth2 consent request:", error)
+      return null
+    })

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/nextjs/src/pages/consent.ts` around lines 76 - 79, Update the OAuth2
consent request flow in serverSideOAuth2Client().getOAuth2ConsentRequest to log
the caught error before returning null. Preserve the existing graceful null
fallback while ensuring failures, including configuration and network errors,
include useful error details in the established logging mechanism.

packages/nextjs/src/pages/session.ts (1)

47-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Simplify state updates with finally.

You can use a .finally() block to handle setting the loading state, reducing duplication across the success and error paths.

♻️ Proposed refactor

       .then((session) => {
         setSession(session)
-        setLoading(false)
-        return
       })
       .catch((err: unknown) => {
         setError(err instanceof Error ? err : new Error(String(err)))
-        setLoading(false)
+      })
+      .finally(() => {
+        setLoading(false)
       })

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/nextjs/src/pages/session.ts` around lines 47 - 55, Refactor the
promise chain in the session-loading flow to move setLoading(false) from both
the success and error handlers into a shared finally block. Keep setSession in
the success handler and setError in the catch handler, preserving their existing
behavior.

packages/nextjs/src/app/consent.test.ts (1)

82-161: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Add tests for the identity-match/mismatch branch.

No test in either describe block passes identityId, so the "Forbidden: Session identity does not match..." guard (the security-critical part of both functions) is never exercised. The mocks here also don't stub getOAuth2ConsentRequest, so this branch would currently throw if invoked.

Add cases for: (1) matching identityId proceeds to accept/reject, (2) mismatched identityId throws.

Also applies to: 163-208

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/nextjs/src/app/consent.test.ts` around lines 82 - 161, Extend both
consent test describe blocks covering acceptConsentRequest and
rejectConsentRequest with identityId cases: stub getOAuth2ConsentRequest with a
request identity for each test, verify matching identityId proceeds to the
underlying accept/reject call, and verify mismatched identityId throws the
forbidden error without proceeding. Keep assertions focused on the security
guard and ensure the mocks support the lookup path.

packages/elements-react/src/theme/default/utils/oauth2-config.ts (1)

4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use a relative import for internal types.

Importing from the package's own public entry point (@ory/elements-react) from within its internal source code can cause module resolution or circular dependency issues during the build step. Please use a relative path instead.

♻️ Proposed fix

-import { OryClientConfiguration } from "`@ory/elements-react`"
+import { OryClientConfiguration } from "../../../util/clientConfiguration"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/elements-react/src/theme/default/utils/oauth2-config.ts` at line 4,
Update the OryClientConfiguration import in oauth2-config.ts to use the
appropriate relative internal source path instead of the package public entry
point, preserving the existing type usage.

packages/nextjs/src/utils/rewrite.ts (1)

82-85: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Handle top-level arrays safely to prevent type conversion issues.

If obj is an Array (e.g., a top-level JSON array response), allowing it to fall through to Object.fromEntries(Object.entries(obj)...) will incorrectly convert it into an Object with stringified numeric keys (e.g., {"0": item1, "1": item2}).

Consider adding early handling for arrays to prevent accidental data corruption.

♻️ Proposed refactor

   // Handle null/undefined input to prevent runtime errors
   if (!obj) {
     return obj
   }
+  if (Array.isArray(obj)) {
+    return obj
+      .map((item) =>
+        typeof item === "object" && item !== null
+          ? rewriteJsonResponse(item, proxyUrl)
+          : typeof item === "string" && proxyUrl
+            ? item.replaceAll(orySdkUrl(), proxyUrl)
+            : item,
+      )
+      .filter((item) => item !== undefined) as unknown as T
+  }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/nextjs/src/utils/rewrite.ts` around lines 82 - 85, Update the
null/undefined guard in the object-rewrite logic to detect top-level arrays and
return them unchanged before the Object.fromEntries(Object.entries(...))
conversion. Preserve the existing handling for non-array objects and nullish
inputs.

examples/nextjs-app-router/middleware.ts (1)

34-51: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Add export const config = { matcher: [...] } to scope middleware execution.

Neither middleware file defines a matcher, so per Next.js defaults this middleware (CSRF processing + Ory path routing) runs on every request, including static assets, images, and favicon — unlike the @csrf-armor/nextjs documented setup, which explicitly excludes _next/static, _next/image, and favicon.ico.

• examples/nextjs-app-router/middleware.ts#L34-L51: add a matcher export excluding static asset paths.
• examples/nextjs-app-router-custom-components/middleware.ts#L34-L51: apply the same addition.

♻️ Proposed addition

   const response = NextResponse.next()
   const result = await csrfProtect(request, response)
   if (!result.success) {
     return NextResponse.json(
       { error: "CSRF validation failed" },
       { status: 403 },
     )
   }
   return response
 }
+
+export const config = {
+  matcher: ["/((?!_next/static|_next/image|favicon.ico).*)"],
+}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/nextjs-app-router/middleware.ts` around lines 34 - 51, Add an
exported Next.js middleware config with a matcher that excludes _next/static,
_next/image, and favicon.ico from execution. Apply the same configuration
alongside middleware in examples/nextjs-app-router/middleware.ts and
examples/nextjs-app-router-custom-components/middleware.ts; leave the middleware
request handling unchanged.

examples/nextjs-app-router/app/api/consent/route.ts (1)

82-114: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Validate action explicitly instead of defaulting unknown values to reject.

Any body.action other than exactly "accept" (including missing/malformed values) silently falls through to rejectConsentRequest. This fails safe (denies rather than grants), but it hides malformed-request bugs as spurious consent denials instead of a clear 400.

♻️ Proposed validation

+  if (action !== "accept" && action !== "reject") {
+    return NextResponse.json(
+      { error: "invalid_request", error_description: "Invalid or missing action" },
+      { status: 400 },
+    )
+  }
+
   if (!consentChallenge) {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/nextjs-app-router/app/api/consent/route.ts` around lines 82 - 114,
Validate body.action explicitly before the consent operation: accept only
“accept” or “reject”, and return a 400 invalid_request response for missing or
any other value. Update the branching around acceptConsentRequest and
rejectConsentRequest so rejectConsentRequest is called only when action is
exactly “reject”.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes